Is valid gps input new & setLayout #1402
Conversation
… it back to isValidGPSInput, created to return a message of the error as well as a boolean of whether the GPS Coordinate is valid
… use instead of creating error message within the component
…GPSInputNew merge origin
… Meters, and adapting MapCalibrationInfoDisplayComponents
…rary test function
|
I noticed that the invalid GPS warnings appear only after the user saves the meter. The meter still stores the invalid GPS input. Unless Steve thinks otherwise, I think the warnings should appear as the user is editing the meter. |
|
The pull request also still has text in ( ). |
The potential issue with checking in real time is that you will get errors while typing the GPS value. I think that is why we waited until the save is done. I think it is fine to leave it the way it is. |
| @@ -88,7 +88,8 @@ export default class MapCalibrationInfoDisplayComponent extends React.Component< | |||
| const longitudeIndex = 1; | |||
| if (this.props.currentCartesianDisplay === 'x: undefined, y: undefined') { return; } | |||
| const input = this.state.value; | |||
| if (isValidGPSInput(input)) { | |||
| const {validGps} = isValidGPSInput(input); | |||
| if (validGps) { | |||
There was a problem hiding this comment.
This isn't your doing but would it work if you had an else clause that showed the error if the GPS is invalid? Up to now the code simply does nothing.
I went on a formatting rampage on all file in PR.
There was a problem hiding this comment.
Thanks to @zhenga5 for working on both of these TODO issues. Review and testing found it works fine and addressed the comments by @danielshid.
I cannot comment inside file without changes so here is a thought:
The isValidGPSInput in src/server/services/csvPipeline/uploadMeters.js is very similar to the modified one you changed in src/client/app/utils/calibration.ts. It was not your doing that there are two version but I think it would be good to remove the other one that is scoped to that file and use the modified one from your work.
I also made one other comment. Note both of these were not part of the original issue but related when I looked at the code. If you are willing to make these changes that would be very helpful. If not, then let me know and I can do them.
|
Note I pushed a commit to address formatting issues where most are outside the PR changes but in the same files. |
Description
Changing IsValidGPSInput to include returning a message about why there is something invalid particularly involving GPS inputs whether the input lacks a comma, has too many commas, or the GPS coordinate is out of range. Additionally changed setLayout to a separate utils file and imported it to existing files that use the setLayout function or variations of it.
Type of change
Checklist
Limitations
Shouldn't be any issues regarding IsValidGPSInput. It only changed the return type to {boolean, string}